Repository navigation
Conversation
io.Copy from a reader that fails or ends before any bytes does not write a response. CaptureMetrics treated that ReadFrom as a committed 200, so a later 502 or 204 was recorded as 200. Fixes felixge#42
|
Bonjour, are we sure that this contribution was guided by a real human being? :) 🐳 |
Owner
@jgehrcke IDK about guided, but the PR description was definitely written by an agent. It's not terrible, but nobody would write the test plan like this 😅 . It's been low on my list of things to review as a result. |
|
@felixge looks like we agree :) I submitted the issue, and would actually love to work on a patch myself in the coming days, does that sound alright? So, we can keep the review effort on this patch here somewhat minimal then maybe :) |
Author
|
That's fair. The description is stiffer than I'd send. Please take the patch. I'll close this so it isn't sitting in the queue. |
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CaptureMetricsreported 200 afterio.Copyfrom a reader that failed before any bytes, even when the handler then sent 502. An emptyReadFromfollowed by 204 was recorded as 200 as well.ReadFrommarked the status as written before any byte was copied. A zero-byteReadFromdoes not commit a response, so a laterWriteHeaderis the status. A copy that writes bytes still records 200 and ignores a laterWriteHeader. The client already received 502 or 204 in these cases. Only the captured code was wrong.Test plan
readfrom error then 502fails on the old hook (got=200 want=502) and passes with this change. The client status is 502.readfrom empty then 204fails on the old hook (got=200 want=204) and passes with this change. The client status is 204.go test -race -count=1 .passedFixes #42